Skip to content

Conversation

@olamidepeterojo
Copy link

TODO:

  • Write new tests or update the old ones to cover new functionality.
  • Update doc-strings where appropriate.
  • Update or write new documentation in packit/packit.dev.
  • ‹fill in›

Fixes #882

Related to #866

Merge before/after

RELEASE NOTES BEGIN

Packit now supports automatic ordering of ☕ after all checks pass.

RELEASE NOTES END

@olamidepeterojo
Copy link
Author

My apologies, I forgot to add the license so i will amend and push

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

from datetime import datetime
from typing import Any

import requests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using pyforgejo SDK

self.comment = raw.get("comment")
self.uid = raw.get("id")
self.url = raw.get("url")
# Parse timestamps in ISO8601 format (adjust format if needed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI generated?

Comment on lines 55 to 56
self._created = datetime.strptime(raw.get("created"), "%Y-%m-%dT%H:%M:%SZ")
self._edited = datetime.strptime(raw.get("updated"), "%Y-%m-%dT%H:%M:%SZ")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using pyforgejo this can be avoided entirely… it's doing all the parsing by default

f"{project.forge_api_url}/repos/{project.owner}/{project.repo}/commits/"
f"{commit}/statuses"
)
headers = project.get_auth_header() # Get auth headers from project config
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made up method, doesn't exist… there's no reason for it to exist, it is handled by pyforgejo…

def project(service):
return service.get_project(
repo="ogr",
namespace="packit-service",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't exist…

return {"Authorization": "Bearer dummy_token"}


@responses.activate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have this dependency, we're using requre


can also notice the red CI because of that

@softwarefactory-project-zuul
Copy link
Contributor

@lachmanfrantisek
Copy link
Member

Hi @olamidepeterojo !

Thank you for all the contributions. I have a few generic suggestions so your contributions can be accepted more easily.

  • You might want to take a look at our contribution guide that covers a lot. What tools do we use and how we test things.
  • I would suggest looking at other Forgejo contributions to see how you can do things.
  • A note on AI. (As Matěj has pointed out, a few things look really suspicious.;-) Using AI is fine, but it can quite easily hallucinate (e.g. use non-existing properties, API endpoints,...) and might also not fit the project (e.g. introducing another way how we test things.)
  • Also, play with the code locally so you can catch the issues more easily. This helps one better understand the code as well.

Let us know if you need any guidance.

@olamidepeterojo
Copy link
Author

Thank you @lachmanfrantisek. I have taken notes of your suggestions.

@olamidepeterojo
Copy link
Author

Hello @mfocko , could you kindly review for incase there are changes to be made?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement support for Forgejo commit statuses

3 participants